-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix MacOS & Windows compilation #143
Conversation
@larrydewey @DGonzalezVillal |
@heavenboy8 These are some good changes. @DGonzalezVillal is working on a PR which will remove the endpoints which are causing issues for CI, so when that gets finished, I will leave another comment on here, you can rebase, and we will review this and pull in your changes. |
@heavenboy8 will you rebase and we will review and merge? |
Can the commits be squashed before we merge? |
@heavenboy8 we are planning on having a release soon, to have this on release, please rebase and squash commits! |
d7d58ca
to
a951b90
Compare
friendly ping @larrydewey ,@tylerfanelli and @DGonzalezVillal, I did the rebase and squashed the commits so it's ready to be reviewed. Sorry for the delay. |
@ThibsG if you git commit -s then push, does that fix it? |
If that doesn't work, you can also add it manually to the git commit message,
|
Signed-off-by: Thibaud Genty <thibaud.genty@cosmian.com>
Thanks! I didn't know |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. @larrydewey what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes look good to me. In the runners i did notice mac and linux were listed for openssl testing and windows was only listed for no openssl. Does Windows not support openssl? It seems most of it wouldn't be compiled outside of linux anyway
runner: | ||
- ubuntu-latest | ||
- macos-12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does Windows not support Openssl?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SEV library was compiling on MacOS & Windows previously. Recent commits have broken that. This PR fixes it.
Also add MacOS & Windows runners in the CI workflow to keep MacOS & Windows support in the future